Build All Previews and Updated with @Previewable #323
Conversation
|
Hey @mikaelacaron, I’ve addressed the issue as requested. Here are the changes: Feel free to review the updates, and let me know if any adjustments are needed. |
There was a problem hiding this comment.
I have several comments that need resolved please. If you have any questions, you can comment directly on that thread, otherwise click the resolve button once you've addressed my comment. Once you fix everything, please click the re-review button
Notice also how it's now that this PR isn't accurate because of removing Firebase stuff, that's not noted in the PR title or what this issue was about. Mixing multiple things in PRs makes it harder to tell the history over time.
Remove all the firebase related changes and redo the Previewable changes please (this may be easier to just make a new branch and PR, otherwise you'll get lots of changes cause Xcode moves things around.
Also make a new issue about this deprecation and where it's documented
| FF153AFF2B07C3E000D0BA30 /* FirebaseCrashlytics in Frameworks */ = {isa = PBXBuildFile; productRef = FF153AFE2B07C3E000D0BA30 /* FirebaseCrashlytics */; }; | ||
| FF218EF62B00865F0025A533 /* AnalyticsService.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF218EF52B00865F0025A533 /* AnalyticsService.swift */; }; | ||
| FF3DDF522AA4D28F009D91C4 /* DashboardViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF3DDF512AA4D28F009D91C4 /* DashboardViewModel.swift */; }; | ||
| FF4E82BE2AD39863004949AF /* FirebaseRemoteConfig in Frameworks */ = {isa = PBXBuildFile; productRef = FF4E82BD2AD39863004949AF /* FirebaseRemoteConfig */; }; | ||
| FF4E82C02AD39863004949AF /* FirebaseRemoteConfigSwift in Frameworks */ = {isa = PBXBuildFile; productRef = FF4E82BF2AD39863004949AF /* FirebaseRemoteConfigSwift */; }; | ||
| FF4E82C22AD39863004949AF /* FirebaseStorage in Frameworks */ = {isa = PBXBuildFile; productRef = FF4E82C12AD39863004949AF /* FirebaseStorage */; }; |
There was a problem hiding this comment.
Don't do this in this PR, can you create a separate issue and PR for why this is necessary?
Several packages are remove and I'm unsure why
| isa = XCRemoteSwiftPackageReference; | ||
| repositoryURL = "https://github.com/firebase/firebase-ios-sdk.git"; | ||
| requirement = { | ||
| kind = upToNextMajorVersion; | ||
| minimumVersion = 10.14.0; | ||
| minimumVersion = 11.3.0; |
There was a problem hiding this comment.
also don't update packages, this is something that should explicitly be in it's own PR, not mixed with other things
again, make a new issue / PR for this (but just make an issue, this is something that someone else can do as well to contribute to the project
| @@ -6,7 +6,7 @@ | |||
| // | |||
|
|
|||
| import Foundation | |||
| import FirebaseFirestoreSwift | |||
| import FirebaseFirestore | |||
There was a problem hiding this comment.
Please put all the packages back to the way they were and then I can review the rest of the files
|
ok @mikaelacaron I got it thanks , so can I just edit the current project.pbxproj as it was before , sure I will but this packages back also , I didn't get "redo the Previewable changes please" like if is there something wrong in this change for @Previewable ? |
I didn't read them because there was too much going on with all the package stuff Yes you could change the pbxproj file manually, but it would be faster and easier to just branch from |
oh ok got it , so should I create new PR ? if so then I will close this current PR |
|
@Yashraj49 Yes, I think it will be easiest to close this PR and create a new one create a new branch from and make an issue about what's deprecated, link to the docs, and why we need to change it |
Yashraj49
deleted the
#309-Build-All-Previews-and-Updated-with-@Previewable
branch
What it Does
@Previewable#309How I Tested